Adding di.cache module #76
Conversation
di/cache/init.q
Outdated
| export:([ | ||
| getperf:getperf; | ||
| sdd:add; | ||
| drop:drop; | ||
| execute:execute | ||
| ]) |
There was a problem hiding this comment.
| export:([ | |
| getperf:getperf; | |
| sdd:add; | |
| drop:drop; | |
| execute:execute | |
| ]) | |
| export:([getperf;add;drop;execute]) |
assuming sdd was a typo
di/cache/cache.q
Outdated
| / return timestamp function | ||
| cp:{.z.p}; |
There was a problem hiding this comment.
there doesn't appear to be a way to change this for end-users, so might make more sense to just use .z.p directly instaed of having this function?
di/cache/cache.q
Outdated
|
|
||
| // Drop some ids from the cache | ||
| drop:{[ids] | ||
| ids,:(); |
There was a problem hiding this comment.
inconsistent indentation between different functions in this file, align to style guide please
| / Don't trap the error here - if it throws an error, we want it to be propagated out | ||
| res:value function; | ||
| $[(maxindividual*MB)>size:-22!res; |
There was a problem hiding this comment.
inconsistent indentation
| / the maximum size of the cache in MB | ||
| maxsize:10; | ||
|
|
||
| / the maximum size of any individual result set in MB | ||
| maxindividual:50; | ||
|
|
||
| / make sure the maxindividual isn't bigger than maxsize | ||
| maxindividual:maxsize&maxindividual; |
There was a problem hiding this comment.
we should have a function for users to change these config settings
| / make sure the maxindividual isn't bigger than maxsize | ||
| maxindividual:maxsize&maxindividual; | ||
|
|
||
| MB:2 xexp 20; |
There was a problem hiding this comment.
add a comment to explain this
| / Return the result | ||
| res}; | ||
|
|
||
| // Drop some ids from the cache |
There was a problem hiding this comment.
comments from here onwards use // instead of /
| / check if we need more space to store this item | ||
| [now:cp[]; | ||
| if[0>requiredsize:(maxsize*MB) - size+sum exec size from cache; evict[neg requiredsize;now]]; | ||
| / Insert to the cache table |
There was a problem hiding this comment.
comments should start with lowercase as per style guide
| $[(maxindividual*MB)>size:-22!res; | ||
| / check if we need more space to store this item | ||
| [now:cp[]; | ||
| if[0>requiredsize:(maxsize*MB) - size+sum exec size from cache; evict[neg requiredsize;now]]; | ||
| / Insert to the cache table | ||
| .z.M.cache upsert (id;now;now;size); | ||
| / and insert to the function and results dictionary | ||
| funcs[id]:enlist function; | ||
| results[id]:enlist res; | ||
| / Update the performance | ||
| trackperf[id;status;now]]; | ||
| / Otherwise just log it as an addfail - the result set is too big | ||
| trackperf[id;`fail;cp[]]]; |
There was a problem hiding this comment.
I think it would be worth refactoring this to avoid the code block inside $ https://github.com/DataIntellectTech/kdbx-modules/blob/main/style.md#conditionals-and-execution-control
e.g. something like this, reversing the condition & returning early if the result is too large
| $[(maxindividual*MB)>size:-22!res; | |
| / check if we need more space to store this item | |
| [now:cp[]; | |
| if[0>requiredsize:(maxsize*MB) - size+sum exec size from cache; evict[neg requiredsize;now]]; | |
| / Insert to the cache table | |
| .z.M.cache upsert (id;now;now;size); | |
| / and insert to the function and results dictionary | |
| funcs[id]:enlist function; | |
| results[id]:enlist res; | |
| / Update the performance | |
| trackperf[id;status;now]]; | |
| / Otherwise just log it as an addfail - the result set is too big | |
| trackperf[id;`fail;cp[]]]; | |
| if[(maxindividual*MB)<=size:-22!res; | |
| / log it as an addfail - the result set is too big | |
| trackperf[id;`fail;cp[]]; | |
| :res; | |
| ]; | |
| / check if we need more space to store this item | |
| now:cp[]; | |
| if[0>requiredsize:(maxsize*MB) - size+sum exec size from cache; | |
| evict[neg requiredsize;now]; | |
| ]; | |
| / Insert to the cache table | |
| .z.M.cache upsert (id;now;now;size); | |
| / and insert to the function and results dictionary | |
| funcs[id]:enlist function; | |
| results[id]:enlist res; | |
| / Update the performance | |
| trackperf[id;status;now]]; |
| / Load core functionality into root module namespace | ||
| \l ::cache.q | ||
|
|
||
| export:([getperf;add;drop;execute]) |
There was a problem hiding this comment.
I don't think add or drop should be exposed to end users, I don't think we'd expect them to be using those functions directly
Commiting code from Adam Beck and Lewis Cooley